Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use TS satisfies #8032

Merged
merged 3 commits into from
Jul 11, 2023
Merged

use TS satisfies #8032

merged 3 commits into from
Jul 11, 2023

Conversation

turadg
Copy link
Member

@turadg turadg commented Jul 10, 2023

Description

Makes use of @satisfies for the OfferMaker lookup in clientSupport. Also bumps deps to support it.

See https://devblogs.microsoft.com/typescript/announcing-typescript-5-0/#satisfies-support-in-jsdoc

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

CI

@turadg turadg requested review from mhofman and dckc July 10, 2023 19:33
@turadg turadg enabled auto-merge July 10, 2023 19:33
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onward

* brand: import('@agoric/internal/src/marshal.js').BoardRemote & Brand,
* brand: import('@agoric/internal/src/marshal.js').BoardRemote,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Losing Brand is a bummer.

I have come to realize that the unmarshalled objects don't need to know their board id - the correspondence can be kept elsewhere. That's the approach I'm using in 2d7a2c5 in #7939.

But I suppose the current code relies on getBoardId(), so BoardRemote is the relevant type.

Comment on lines 12 to 14
// NB: not really a Proposal because the brands are not remotes
// Instead they're copyRecord like "{"boardId":"board0257","iface":"Alleged: IST brand"}" to pass through the boardId
// mustMatch(harden(proposal), ProposalShape);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This NB: is no longer the case, is it? makeBoardRemote uses Far. The change to smallcaps made it necessary, IIRC.

@@ -373,6 +369,9 @@ const makePushPriceOffer = (_agoricNames, opts, previousOffer) => {
};
};

/**
* @satisfies {Record<string, Record<string, import('@agoric/smart-wallet/src/types.js').OfferMaker>>}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL:

satisfies makes sure a value's type matches some type shape, but doesn't widen the type unnecessarily.

@turadg turadg added this pull request to the merge queue Jul 11, 2023
Merged via the queue into master with commit 648d42f Jul 11, 2023
@turadg turadg deleted the ta/use-satisfies branch July 11, 2023 00:54
toliaqat added a commit that referenced this pull request Jul 12, 2023
This PR removes the following lint warnings introduced by
version update of eslint-plugin-jsdoc in #8032.

(jsdoc/no-defaults) Defaults are not permitted on @param
mhofman pushed a commit that referenced this pull request Aug 7, 2023
mhofman pushed a commit that referenced this pull request Aug 7, 2023
This PR removes the following lint warnings introduced by
version update of eslint-plugin-jsdoc in #8032.

(jsdoc/no-defaults) Defaults are not permitted on @param
anilhelvaci pushed a commit to anilhelvaci/agoric-sdk that referenced this pull request Aug 16, 2023
This PR removes the following lint warnings introduced by
version update of eslint-plugin-jsdoc in Agoric#8032.

(jsdoc/no-defaults) Defaults are not permitted on @param
anilhelvaci pushed a commit to Jorge-Lopes/agoric-sdk that referenced this pull request Feb 16, 2024
This PR removes the following lint warnings introduced by
version update of eslint-plugin-jsdoc in Agoric#8032.

(jsdoc/no-defaults) Defaults are not permitted on @param
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants